ARROW-10093: [R] Add ability to opt-out of int64 -> int demotion#8341
ARROW-10093: [R] Add ability to opt-out of int64 -> int demotion#8341romainfrancois wants to merge 5 commits into
Conversation
|
Should that be a more general option perhaps ? Is this the right name ? |
nealrichardson
left a comment
nealrichardson
left a comment
There was a problem hiding this comment.
Some suggestions. Looking at https://arrow.apache.org/docs/r/articles/arrow.html#arrow-to-r, I don't see that there's a more general option to have right now. We may want another option around dictionary conversion (https://issues.apache.org/jira/browse/ARROW-7657), but it's not obvious to me that these options would necessarily go together. If we later find that we want to add an umbrella option, we can add it.
There was a problem hiding this comment.
Re: naming, we currently have options "arrow.use_threads" (mostly private but some may need to set it given the multithreading bugs we've observed) and "arrow.dev.repo" (not advertised). I don't feel strongly about the naming convention as long as we're consistent and predictable (which, naturally, names(options()) is not).
Searching around for other conventions, I found package.option_name (cf. tidyverse/dplyr#5548) like we did with arrow.use_threads, so maybe let's standardize on that?
There was a problem hiding this comment.
using arrow.int64_downcast, happy to change
nealrichardson
left a comment
nealrichardson
left a comment
There was a problem hiding this comment.
This looks good, just a couple of quick final notes
There was a problem hiding this comment.
Your other comment said you were going with arrow.int64_downcast but that's not what is here. I'm ok either way though arrow.int64_downcast is more concise (and the "auto" behavior is really about what the default value is).
There was a problem hiding this comment.
Minor thought: should we switch the order of these checks? I have to imagine that GetBoolOption would be faster than doing bounds checking on the full array.
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
nealrichardson
force-pushed
the
ARROW-10093/disable_int64_autoconv
branch
from
2b1269c to
c1b40ec
Compare
2 participants
No description provided.